tests/test_mvcrender.py-71-74 (1)

71-74: String formatting bug: ITERATIONS not interpolated.

The f-string on line 73 has {ITERATIONS} inside a regular string, so it won't be interpolated. This will print the literal text {ITERATIONS} instead of the value 100.

🔎 Proposed fix

 # Test 1: AutoCrop with rotation (most complex operation)
 print(f"\n{'=' * 70}")
-print("Test 1: AutoCrop with rotation ({ITERATIONS} iterations)")
+print(f"Test 1: AutoCrop with rotation ({ITERATIONS} iterations)")
 print(f"{'=' * 70}")

tests/convert_rand_to_hypfer.py-309-312 (1)

309-312: Guard against division by zero.

If there are no segments or all segments are empty, compressed_pixels could be zero, causing a ZeroDivisionError.

🔎 Proposed fix

     print(f"Original pixel data: {original_pixels} values")
     print(f"Compressed pixel data: {compressed_pixels} values")
-    print(f"Compression ratio: {original_pixels / compressed_pixels:.2f}x")
-    print(f"Memory reduction: {(1 - compressed_pixels/original_pixels) * 100:.1f}%")
+    if compressed_pixels > 0 and original_pixels > 0:
+        print(f"Compression ratio: {original_pixels / compressed_pixels:.2f}x")
+        print(f"Memory reduction: {(1 - compressed_pixels/original_pixels) * 100:.1f}%")
+    else:
+        print("No pixel data to calculate compression ratio")

backups/drawable_ori.py-346-415 (1)

346-415: Inconsistent coordinate ordering between _filled_circle and _filled_circle_optimized.

These two similar methods use opposite coordinate conventions:

• _filled_circle (line 358): y, x = center
• _filled_circle_optimized (line 399): x, y = center

This inconsistency is error-prone and could cause subtle bugs if a caller switches between these methods.

backups/drawable_ori.py-34-40 (1)

34-40: Comment color description doesn't match the actual color value.

The comment says "Red color for error messages" for ERROR_OUTLINE, but the color value (0, 0, 0, 255) is black (RGBA). Similarly, ERROR_COLOR comment says "Red color with lower opacity for error outlines" which is inconsistent with the first comment.

🔎 Proposed fix

-    ERROR_OUTLINE: Color = (0, 0, 0, 255)  # Red color for error messages
-    ERROR_COLOR: Color = (
+    ERROR_OUTLINE: Color = (0, 0, 0, 255)  # Black color for error outlines
+    ERROR_COLOR: Color = (
         255,
         0,
         0,
         191,
-    )  # Red color with lower opacity for error outlines
+    )  # Red color with lower opacity for error fill

new_tests/TEST_RESULTS_SUMMARY.md-6-11 (1)

6-11: Documentation inconsistency: statistics conflict with detailed sections.

The summary states "131 (100%) ✅" passing, but subsequent sections (lines 25-57) describe multiple failing tests with specific counts (e.g., "1 Failure" in test_shared.py, "4 Failures" in test_drawable.py). Either update the statistics to reflect actual state or clarify that the numbers represent tests after fixes were applied.

new_tests/IMPLEMENTATION_SUMMARY.md-23-29 (1)

23-29: Documentation conflict with TEST_RESULTS_SUMMARY.md.

This file reports 78% passing (108/138), while TEST_RESULTS_SUMMARY.md claims 100% passing (131/131). Consider reconciling these documents or adding timestamps to clarify they represent different points in time.

tests/rand_rooms_test.py-232-233 (1)

232-233: Variable shadowing: room_data has two different meanings.

Line 232 assigns room_data = dest_json.get("rooms", []) (a list of room info), then line 265 assigns room_data = {...} (a single room's properties). This shadowing can cause confusion and potential bugs.

🔎 Proposed fix

         # Process destinations JSON to get room names
         dest_json = destinations
-        room_data = dest_json.get("rooms", [])
-        room_id_to_data = {room["id"]: room for room in room_data}
+        dest_rooms = dest_json.get("rooms", [])
+        room_id_to_data = {room["id"]: room for room in dest_rooms}

Also applies to: 265-271

tests/rooms_test.py-71-125 (1)

71-125: os.makedirs with empty directory path will fail.

Line 121 calls os.makedirs(os.path.dirname(test_data_path), exist_ok=True) where test_data_path = "test.json". Since os.path.dirname("test.json") returns an empty string, this will raise an error on some systems.

🔎 Proposed fix

-        os.makedirs(os.path.dirname(test_data_path), exist_ok=True)
+        dir_name = os.path.dirname(test_data_path)
+        if dir_name:
+            os.makedirs(dir_name, exist_ok=True)

tests/test_hypfer_profiling.py-243-251 (1)

243-251: Potential NameError for cpu_profiler in finally block.

If an exception occurs before line 193 assigns cpu_profiler, the reference on line 247 will raise NameError. Initialize it before the try block:

         start_time = time.time()
+        cpu_profiler = None
         if self.profiler:
             self.profiler.take_memory_snapshot("Before Image Generation")
             cpu_profiler = self.profiler.start_cpu_profile("Hypfer Image Generation")

Committable suggestion skipped: line range outside the PR's diff.

backups/hypfer_rooms_handler.py-1-6 (1)

1-6: Typo: "Hipfer" should be "Hypfer".

The module docstring and class docstring use "Hipfer" instead of "Hypfer" consistently. This should match the naming convention used elsewhere in the codebase.

 """
-Hipfer Rooms Handler Module.
-Handles room data extraction and processing for Valetudo Hipfer vacuum maps.
+Hypfer Rooms Handler Module.
+Handles room data extraction and processing for Valetudo Hypfer vacuum maps.

backups/rand25_handler_rooms.py-97-99 (1)

97-99: Return type annotation is incorrect.

The method signature declares RoomsProperties as return type, but the actual return on line 164 is tuple[RoomsProperties, zone_properties, point_properties] (a 3-tuple).

🔎 Proposed fix

    async def extract_room_properties(
        self, json_data: JsonType, destinations: JsonType
-    ) -> RoomsProperties:
+    ) -> tuple[RoomsProperties, Any, Any]:
        """Extract the room properties."""

tests/debug_manage_rooms.py (1)

13-16: Consider improving error handling for better debugging experience.

The current exception handling could be enhanced:

1. Errors should be printed to stderr rather than stdout (standard convention).
2. The script should exit with a non-zero code on failure for better integration with automation tools.

🔎 Proposed improvements

 try:
     asyncio.run(main())
 except Exception:
-    traceback.print_exc(file=sys.stdout)
+    traceback.print_exc(file=sys.stderr)
+    sys.exit(1)

tests/PROFILING_README.md (1)

54-81: Add language specifiers to fenced code blocks.

The markdown linter flags missing language specifiers for fenced code blocks at lines 54, 67, and 76. These appear to be example output text.

🔎 Proposed fix to add language specifiers

-```
+```text
 🔍 Memory Usage Timeline:

-```
+```text
 ⚡ CPU USAGE ANALYSIS

-```
+```text
 ⏱️  TIMING ANALYSIS

tests/VALETUDO_MAP_PARSER_TYPES_USAGE_REPORT.md (1)

1-373: Documentation provides useful reference for library usage patterns.

This comprehensive usage report documents the valetudo_map_parser types well. A few minor markdown linting improvements could be made:

1. Tables (lines 226-260) should be surrounded by blank lines per MD058
2. The fenced code block at line 337 should have a language specifier (e.g., text or plaintext)
3. Line 372 uses emphasis instead of a heading (MD036)

These are cosmetic and don't affect the content's utility.

tests/analyze_room_connections.py (2)

104-127: Brute-force distance calculation may be slow for large rooms.

The nested loops computing minimum distance have O(n×m) complexity where n and m are pixel counts. For rooms with thousands of pixels, this can be quite slow. Consider using scipy.spatial.distance.cdist for vectorized computation.

🔎 Proposed optimization using cdist

+from scipy.spatial.distance import cdist
+
 if len(room2_indices[0]) > 0 and len(room7_indices[0]) > 0:
-    # Calculate the minimum distance between Room 2 and Room 7
-    min_distance = float("inf")
-    closest_point_room2 = None
-    closest_point_room7 = None
-
-    for i in range(len(room2_indices[0])):
-        y2, x2 = room2_indices[0][i], room2_indices[1][i]
-        for j in range(len(room7_indices[0])):
-            y7, x7 = room7_indices[0][j], room7_indices[1][j]
-            distance = np.sqrt((x2 - x7) ** 2 + (y2 - y7) ** 2)
-            if distance < min_distance:
-                min_distance = distance
-                closest_point_room2 = (x2, y2)
-                closest_point_room7 = (x7, y7)
+    # Vectorized distance calculation
+    coords2 = np.column_stack((room2_indices[1], room2_indices[0]))  # (x, y)
+    coords7 = np.column_stack((room7_indices[1], room7_indices[0]))
+    distances = cdist(coords2, coords7)
+    min_idx = np.unravel_index(np.argmin(distances), distances.shape)
+    min_distance = distances[min_idx]
+    closest_point_room2 = (coords2[min_idx[0], 0], coords2[min_idx[0], 1])
+    closest_point_room7 = (coords7[min_idx[1], 0], coords7[min_idx[1], 1])

---

152-154: Unused output directory creation.

The output directory is created but never used. The commented-out matplotlib import suggests visualization was planned. Consider removing this dead code or implementing the visualization.

-    # Create a text-based visualization of the rooms
-    output_dir = os.path.join(script_dir, "output")
-    os.makedirs(output_dir, exist_ok=True)

tests/test_mvcrender.py (1)

10-22: Non-Pythonic semicolons in class definition.

The semicolons at the end of statements (lines 14, 15, 18, 20) are unnecessary in Python and reduce readability.

🔎 Proposed cleanup

 class DummyShared:
     def __init__(self):
         self.trims = type("T", (),
                           {"to_dict": lambda self: {"trim_up": 0, "trim_down": 0, "trim_left": 0, "trim_right": 0}})()
-        self.offset_top = 0;
-        self.offset_down = 0;
-        self.offset_left = 0;
+        self.offset_top = 0
+        self.offset_down = 0
+        self.offset_left = 0
         self.offset_right = 0
-        self.vacuum_state = "cleaning";
+        self.vacuum_state = "cleaning"
         self.image_auto_zoom = True
-        self.image_ref_width = 0;
+        self.image_ref_width = 0
         self.image_ref_height = 0

tests/test_rand_to_hypfer_compression.py (2)

14-67: Compression function is well-implemented with correct run-length encoding.

The algorithm correctly groups consecutive x pixels on the same row into runs. The coordinate transformation and run building logic are sound.

Minor note: prev_y (line 61) is assigned but never used since row changes are detected via y == run_y.

🔎 Remove unused variable

-        prev_x, prev_y = x, y
+        prev_x = x

---

70-76: Import os should be at module level.

Moving the os import to the top of the file with other imports follows PEP 8 conventions and makes dependencies clearer.

 """
 import json
+import os


 def compress_rand_to_hypfer(
     ...

 def main():
     """Test the compression on segment 20 from rand.json."""
-
-    # Load rand.json
-    import os
     script_dir = os.path.dirname(os.path.abspath(__file__))

tests/benchmark_margins.py (1)

99-101: Unused auto_crop instance.

The AutoCrop instance is created but never used in the benchmark. Either remove it or use it for benchmarking the actual AutoCrop methods.

         # Create AutoCrop instance
         handler = DummyHandler()
-        auto_crop = AutoCrop(handler)

tests/analyze_room12.py (3)

25-26: Add error handling for missing test data file.

Unlike analyze_segment_walls.py which checks if the file exists before opening, this script will raise an unhandled FileNotFoundError if test.json is missing.

🔎 Proposed fix

+    if not os.path.exists(test_data_path):
+        _LOGGER.error(f"Test data file not found: {test_data_path}")
+        return
+
     with open(test_data_path, "r", encoding="utf-8") as file:
         data = json.load(file)

---

79-79: Move import to top of file for clarity.

Importing scipy.ndimage inside the function is unusual. For a dedicated analysis script, moving this import to the top-level (alongside numpy) would fail fast if the dependency is missing and improve readability.

---

100-106: Consider documenting the magic number thresholds.

The hardcoded coordinate thresholds (3350, 3345, 2540, 2535) lack context. A brief comment explaining what these values represent would help future maintainers understand the debugging intent.

tests/analyze_segment_walls.py (2)

126-134: Consider optimizing adjacency search with set-based lookup.

The nested loop over segment_coords and wall_coords has O(n*m) complexity. For large maps, this could be slow. Converting wall_coords to a set and checking the 8-neighborhood would reduce complexity to O(n).

🔎 Proposed optimization

     # Find segment pixels that are adjacent to wall pixels
+    wall_set = set(wall_coords)
     for sx, sy in segment_coords:
-        for wx, wy in wall_coords:
-            # Check if the segment pixel is adjacent to the wall pixel
-            if abs(sx - wx) <= 1 and abs(sy - wy) <= 1:
-                adjacent_pairs.append(((sx, sy), (wx, wy)))
-                break
+        # Check 8-neighborhood for wall pixels
+        for dx in (-1, 0, 1):
+            for dy in (-1, 0, 1):
+                neighbor = (sx + dx, sy + dy)
+                if neighbor in wall_set:
+                    adjacent_pairs.append(((sx, sy), neighbor))
+                    break
+            else:
+                continue
+            break
 
     return adjacent_pairs

---

60-65: Triplet bounds check is correct but could use <=.

The condition i + 2 < len(compressed_pixels) works but is subtly confusing. Using i + 3 <= len(compressed_pixels) or simply relying on the range would be clearer, since you need indices i, i+1, and i+2 to all be valid.

tests/convert_rand_to_hypfer.py (2)

17-18: Remove unused variable prev_y.

The variable prev_y is assigned on line 33 but never read. It can be removed to avoid confusion.

🔎 Proposed fix

-    prev_x = prev_y = None
+    prev_x = None
     run_start_x = run_y = None

And on line 33:

-        prev_x, prev_y = x, y
+        prev_x = x

Also applies to: 33-33

---

46-55: Consider incremental computation to reduce memory usage.

Expanding all pixel runs into individual coordinates (lines 52-55) can consume significant memory for large segments. The min/max/avg statistics could be computed incrementally without storing all coordinates.

tests/tests/test_robot_angles.py (1)

1-6: Consider renaming to avoid pytest collection.

This script uses print-based output rather than assertions and is not a proper pytest test. The test_ prefix in the filename and function names may cause pytest to attempt collection. Consider renaming to analyze_robot_angles.py or similar.

backups/test_old_pars.py (1)

53-107: Recursive parsing could cause stack overflow on malformed data.

parse_block uses tail recursion (line 107) to process sequential blocks. With malformed data containing very small or zero-length blocks, this could exceed Python's recursion limit. Consider an iterative approach if this code is used in production.

🔎 Iterative alternative

     @staticmethod
     def parse_block(
         buf: bytes,
         offset: int,
         result: Optional[Dict[int, Any]] = None,
         pixels: bool = False,
     ) -> Dict[int, Any]:
         """Parse a block of data from the map data."""
         result = result or {}
-        if len(buf) <= offset:
-            return result
-
-        # ... parsing logic ...
-
-        return RRMapParser.parse_block(buf, offset + length + hlength, result, pixels)
+        while offset < len(buf):
+            # ... parsing logic ...
+            offset = offset + length + hlength
+        return result

backups/drawable_ori.py (1)

470-476: Unused grid allocation and slow nested loop despite docstring claiming vectorized operations.

Line 471 creates y_indices, x_indices using np.mgrid but they are never used - the code immediately iterates with nested Python loops calling point_inside for each pixel. This is both wasteful (allocates unused arrays) and slow (O(n×m×k) where k is polygon vertices).

🔎 Proposed fix - remove unused allocation

             # Create a mask for the polygon region
             mask = np.zeros((max_y - min_y + 1, max_x - min_x + 1), dtype=bool)

             # Adjust points to the mask's coordinate system
             adjusted_points = [(p[0] - min_x, p[1] - min_y) for p in points]

-            # Create a grid of coordinates and use it to test all points at once
-            y_indices, x_indices = np.mgrid[0 : mask.shape[0], 0 : mask.shape[1]]
-
             # Test each point in the grid
             for i in range(mask.shape[0]):
                 for j in range(mask.shape[1]):
                     mask[i, j] = Drawable.point_inside(j, i, adjusted_points)

new_tests/IMPLEMENTATION_SUMMARY.md (1)

161-165: Remove hardcoded local path for portability.

The command includes a user-specific path /Users/sandro/PycharmProjects/.... Use a relative path or generic placeholder.

🔎 Suggested fix

 ### Run All Tests
 ```bash
-cd /Users/sandro/PycharmProjects/Python-package-valetudo-map-parser
-.venv/bin/python -m pytest new_tests/
+# From repository root:
+python -m pytest new_tests/
 ```

backups/refactored_old_code.py (1)

1-44: Consider removing backup file from version control.

This file contains only commented-out legacy code. Version control history already preserves deleted code, making this backup redundant. Consider removing it or adding the backups/ directory to .gitignore.

tests/test_parser.py (3)

10-16: Path manipulation and import approach is fragile.

The sys.path.insert(0, str(scr_path)) combined with imports from backups and valetudo_map_parser.config directories makes this test dependent on a specific directory structure. Consider using a proper package setup or pytest fixtures for cleaner test isolation.

---

31-53: Hardcoded payload filename and early silent return may cause test failures to go unnoticed.

The hardcoded filename map_data_20250728_185945.bin and the early return without raising an exception means if the file is missing, the test silently passes. For a test file, consider raising an assertion or using pytest.skip() to make missing test data explicit.

🔎 Suggested improvement

     if not payload_path:
         print(f"Could not find payload file: {payload_file}")
         print("Tried these locations:")
         for path in possible_paths:
             print(f"  - {path}")
-        return
+        raise FileNotFoundError(f"Could not find payload file: {payload_file}")

---

198-205: Hardcoded output path may fail if tests/ directory doesn't exist.

The output file path tests/test_rand256.json assumes the tests/ directory exists. Consider using os.makedirs with exist_ok=True or writing relative to __file__.

🔎 Suggested fix

     # Save results to JSON file
-    output_file = "tests/test_rand256.json"
+    output_file = Path(__file__).parent / "test_rand256.json"
     try:
+        output_file.parent.mkdir(parents=True, exist_ok=True)
         with open(output_file, "w") as f:
             json.dump(results, f, indent=2, default=str)

tests/test_room_store.py (2)

49-136: Missing cleanup of RoomStore singleton instances between test runs.

The RoomStore singleton instances created during test_room_store_singleton() persist into test_room_store_edge_cases() and subsequent test runs. This can cause test pollution and flaky tests. Consider adding a cleanup mechanism.

🔎 Suggested improvement

+def cleanup_room_store():
+    """Clear all RoomStore instances for test isolation."""
+    RoomStore._instances.clear()
+
+
 def test_room_store_singleton():
     """Test that RoomStore maintains singleton per vacuum_id."""
+    cleanup_room_store()
     _LOGGER.info("=" * 60)

---

260-262: Tests run sequentially without pytest integration.

Running tests via if __name__ == "__main__" bypasses pytest's test discovery, fixtures, and reporting. Consider converting to proper pytest test functions or adding pytest markers.

tests/debug_binary.py (1)

162-168: Silent failure when payload file is not found.

The script silently returns when the payload file doesn't exist, which could lead to confusion. Consider adding a more explicit error or usage message.

tests/test_status_text_performance.py (1)

10-11: Duplicate import of sys module.

sys is imported on line 10 and again on line 114. Remove the duplicate import on line 114.

tests/compare_payloads.py (1)

93-105: Hardcoded payload paths reduce test portability.

The payload file paths are hardcoded with specific filenames and relative paths. Consider using a configuration file or environment variable for test data locations.

tests/rand_rooms_test.py (2)

125-197: Unused method _process_segment_data is dead code.

The _process_segment_data method is defined but never called anywhere in the file. The room processing logic in async_extract_room_properties (lines 236-278) duplicates much of this functionality. Consider either removing this dead code or refactoring to use it.

---

360-368: Deprecated asyncio pattern for running async code.

The asyncio.new_event_loop() / set_event_loop() / run_until_complete() pattern is deprecated. Use asyncio.run() instead.

🔎 Proposed fix

 def __main__():
     """Main function."""
-    loop = asyncio.new_event_loop()
-    asyncio.set_event_loop(loop)
-
-    try:
-        loop.run_until_complete(test_rand_rooms_handler())
-    finally:
-        loop.close()
+    asyncio.run(test_rand_rooms_handler())

tests/rooms_test.py (1)

137-153: Missing exception handling in convex_hull_outline compared to similar implementation.

The convex_hull_outline function in rand_rooms_test.py (lines 94-123) includes a try-except block with a bounding box fallback. This implementation lacks that protection, which could cause uncaught exceptions for degenerate point configurations.

🔎 Suggested improvement

 def convex_hull_outline(mask: np.ndarray) -> list[tuple[int, int]]:
     y_indices, x_indices = np.where(mask > 0)
     if len(x_indices) == 0 or len(y_indices) == 0:
         return []

     points = np.column_stack((x_indices, y_indices))
     if len(points) < 3:
         return [(int(x), int(y)) for x, y in points]

-    hull = ConvexHull(points)
-    hull_points = [
-        (int(points[vertex][0]), int(points[vertex][1])) for vertex in hull.vertices
-    ]
-    if hull_points[0] != hull_points[-1]:
-        hull_points.append(hull_points[0])
-    return hull_points
+    try:
+        hull = ConvexHull(points)
+        hull_points = [
+            (int(points[vertex][0]), int(points[vertex][1])) for vertex in hull.vertices
+        ]
+        if hull_points[0] != hull_points[-1]:
+            hull_points.append(hull_points[0])
+        return hull_points
+    except Exception:
+        # Fallback to bounding box
+        x_min, y_min = np.min(points, axis=0)
+        x_max, y_max = np.max(points, axis=0)
+        return [
+            (int(x_min), int(y_min)),
+            (int(x_max), int(y_min)),
+            (int(x_max), int(y_max)),
+            (int(x_min), int(y_max)),
+            (int(x_min), int(y_min)),
+        ]

tests/test_floor_data.py (2)

6-27: Consider importing actual classes instead of replicating them.

The test replicates TrimsData and FloorData locally rather than importing from valetudo_map_parser.config.types. This creates a risk that tests pass while the actual implementation differs. If the goal is to test the public API exports (per PR objectives), import the real types:

-# Replicate TrimsData for testing
-@dataclass
-class TrimsData:
-    floor: str = ""
-    trim_up: int = 0
-    trim_left: int = 0
-    trim_down: int = 0
-    trim_right: int = 0
-
-    @classmethod
-    def from_list(cls, crop_area: List[int], floor: Optional[str] = None):
-        return cls(
-            trim_up=crop_area[0],
-            trim_left=crop_area[1],
-            trim_down=crop_area[2],
-            trim_right=crop_area[3],
-            floor=floor or "",
-        )
-
-    def to_dict(self):
-        return asdict(self)
+from valetudo_map_parser import FloorData, TrimsData

This also validates that the public exports work correctly.

---

212-231: Consider using pytest for consistency with the test suite.

This file uses a custom test runner with print statements, while other tests in new_tests/ use pytest. Converting to pytest would provide better CI integration, clearer failure reporting, and consistency.

🔎 Example conversion

import pytest
from valetudo_map_parser import FloorData, TrimsData

def test_trims_data_from_list():
    """Test TrimsData.from_list() with crop_area."""
    crop_area = [790, 490, 3209, 2509]
    trims = TrimsData.from_list(crop_area, floor="Ground Floor")
    
    assert trims.trim_up == 790
    assert trims.trim_left == 490
    # ... etc

tests/test_hypfer_profiling.py (2)

21-26: Fragile path manipulation for imports.

Using sys.path.insert with relative paths is brittle and can break depending on the working directory. Consider using package-relative imports or configuring PYTHONPATH in test setup instead.

-# Add the parent directory to the path so we can import the modules
-sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
-
-from SCR.valetudo_map_parser.config.shared import CameraSharedManager
-from SCR.valetudo_map_parser.hypfer_handler import HypferMapImageHandler
+from valetudo_map_parser.config.shared import CameraSharedManager
+from valetudo_map_parser.hypfer_handler import HypferMapImageHandler

This requires proper package installation (e.g., pip install -e .).

---

259-277: Use asyncio.run() for cleaner event loop handling.

The manual event loop creation pattern is verbose and outdated. Modern Python (3.7+) provides asyncio.run() which handles loop lifecycle automatically.

🔎 Suggested refactor

-    loop = asyncio.new_event_loop()
-    asyncio.set_event_loop(loop)
-
     # Legacy cProfile for compatibility
     profiler = cProfile.Profile()
     profiler.enable()
 
     try:
         if test.profiler:
             test.profiler.take_memory_snapshot("Test Start")
 
-        loop.run_until_complete(test.test_image_handler())
+        asyncio.run(test.test_image_handler())
 
         if test.profiler:
             test.profiler.take_memory_snapshot("Test Complete")
 
     finally:
         profiler.disable()
-        loop.close()

backups/hypfer_rooms_handler.py (2)

68-93: Consider making rdp non-async for better performance.

The rdp method is marked async but performs no actual async I/O—only recursive calls. This adds unnecessary coroutine overhead. For CPU-bound recursive algorithms like RDP, a regular function is more efficient:

-    async def rdp(
+    def rdp(
         self, points: List[Tuple[int, int]], epsilon: float
     ) -> List[Tuple[int, int]]:
         """Ramer-Douglas-Peucker algorithm for simplifying a curve."""
         if len(points) <= 2:
             return points
         # ... 
         if dmax > epsilon:
-            first_segment = await self.rdp(points[: index + 1], epsilon)
-            second_segment = await self.rdp(points[index:], epsilon)
+            first_segment = self.rdp(points[: index + 1], epsilon)
+            second_segment = self.rdp(points[index:], epsilon)

If kept async, note that deep recursion on complex contours could hit Python's recursion limit.

---

364-379: Potential edge case in point_in_polygon with degenerate edges.

If an edge has p1x == p2x and p1y != p2y, xinters is never assigned but used on line 376. The current condition flow should prevent this, but the logic is fragile. Consider a cleaner guard:

                         if p1y != p2y:
                             xinters = (y - p1y) * (p2x - p1x) / (p2y - p1y) + p1x
-                        if p1x == p2x or x <= xinters:
+                            if x <= xinters:
+                                inside = not inside
+                        elif p1x == p2x:  # Vertical edge
                             inside = not inside

This makes the vertical edge case explicit and avoids relying on xinters state.

backups/hypfer_handler_ori.py (2)

233-237: Unused variable room_element after element map update comment.

The variable room_element is assigned but never used. The comment suggests it should mark the room in the element map, but the implementation is missing.

🔎 Proposed fix

Either implement the element map update or remove the dead code:

-                    # Update element map for this layer
-                    if is_room_layer and 0 < room_id <= 15:
-                        # Mark the room in the element map
-                        room_element = getattr(
-                            DrawableElement, f"ROOM_{room_id}", None
-                        )

---

440-440: Deleting loop variable inside loop iteration is unnecessary.

Deleting calibration_point and rotation_angle at the end of the loop/function has no practical benefit since they go out of scope immediately after the return statement.

🔎 Proposed fix

            for vacuum_point, map_point in zip(vacuum_points, map_points):
                calibration_point = {"vacuum": vacuum_point, "map": map_point}
                calibration_data.append(calibration_point)
-        del vacuum_points, map_points, calibration_point, rotation_angle  # free memory.
        return calibration_data

backups/rand25_handler_rooms.py (2)

263-267: Incomplete implementation marked with pass statement.

The comment indicates this is a simplification and the real implementation would need to identify exact pixels for each room. Consider adding a TODO or implementing the functionality.

Would you like me to help implement the element map update for room pixels, or should this be tracked as a separate issue?

---

430-430: Unnecessary del statement for loop variables.

Deleting room, corners, robot_x, robot_y after return provides no memory benefit as they're already local variables that will be garbage collected.

🔎 Proposed fix

                _LOGGER.debug("%s is in %s", self.file_name, self.robot_in_room["room"])
-                del room, corners, robot_x, robot_y  # free memory.
                return temp

backups/colors.py (2)

270-274: Unnecessary enumerate when index is unused.

The loop uses enumerate but discards the index with _. Simply iterate directly over the zip.

🔎 Proposed fix

            result = []
-            for _, (alpha, rgb) in enumerate(zip(alphas, rgb_colors)):
+            for alpha, rgb in zip(alphas, rgb_colors):
                if rgb is None:
                    result.append((0, 0, 0, int(alpha)))
                else:
                    result.append((rgb[0], rgb[1], rgb[2], int(alpha)))

---

575-580: Potential precision issue in alpha ratio calculation.

The alpha ratio calculation fg_alpha / out_alpha[valid_alpha] can produce unexpected results when out_alpha is very small but above the 0.0001 threshold. Consider using a slightly larger threshold or documenting the precision trade-off.

The current implementation handles the edge case with valid_alpha = out_alpha > 0.0001, which is reasonable for most use cases.

backups/drawable.ori (1)

906-908: Potential AttributeError if path_font is empty string.

When path_font is an empty string "", path_font.endswith("VT.ttf") returns False, but the logic at line 908 assumes path_font is truthy. However, if path_font is None, line 894 would have handled it. The issue is that an empty string passes the if path_font: check but may not be a valid font path.

🔎 Proposed fix

        # Use provided font directly if available; else fall back to default
        user_font = default_font
-        if path_font:
+        if path_font and Path(path_font).exists():
            try:
                user_font = ImageFont.truetype(str(path_font), size)
            except OSError:
                user_font = default_font

backups/map_data_ori.py (2)

19-22: Code duplication: sublist function exists in multiple files.

This exact implementation exists in at least 3 other locations:

• tests/rooms_test.py:132-133
• backups/hypfer_rooms_handler.py:42-43
• tests/rand_rooms_test.py:68-70

Consider extracting to a shared utility module to maintain DRY principles.

🔎 Proposed refactor

Create a shared utility and import it:

# In a shared utils module (e.g., config/utils.py)
def sublist(lst: list, n: int) -> list:
    """Split a list into chunks of specified size."""
    return [lst[i : i + n] for i in range(0, len(lst), n)]

Then import from the shared location in all files that need it.

---

282-283: Use modern Union syntax with | or proper Union type.

The return type hint list or None is not valid Python type annotation syntax. Use list | None (Python 3.10+) or Optional[list].

🔎 Proposed fix

    @staticmethod
-    def get_rrm_goto_predicted_path(json_data: JsonType) -> list or None:
+    def get_rrm_goto_predicted_path(json_data: JsonType) -> list | None:
        """Get the predicted path data from the json."""

Apply the same fix to lines 324, 402, and 492.

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Learnt from: sca075
Repo: sca075/Python-package-valetudo-map-parser PR: 29
File: SCR/valetudo_map_parser/hypfer_draw.py:438-462
Timestamp: 2025-12-19T10:03:44.321Z
Learning: In SCR/valetudo_map_parser/hypfer_draw.py, the `_check_room_with_corners` method correctly sets `self.img_h.robot_in_room` before validation. This state mutation pattern during room iteration is intentional and works correctly with the coordinate system used by the Hypfer handler, similar to the Rand256 handler pattern.

Learnt from: sca075
Repo: sca075/Python-package-valetudo-map-parser PR: 29
File: SCR/valetudo_map_parser/rand256_handler.py:445-461
Timestamp: 2025-12-19T06:48:40.379Z
Learning: In SCR/valetudo_map_parser/rand256_handler.py, the bounding box comparison `self.robot_in_room["right"] <= int(robot_x) <= self.robot_in_room["left"]` is correct for the Rand256 coordinate system, which uses inverted or special axis conventions compared to standard Cartesian coordinates. The "left" value can be larger than "right" in this coordinate system.